Skip to content

feat: validate space strategies #513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 58 commits into
base: master
Choose a base branch
from

Conversation

wa0x6e
Copy link
Contributor

@wa0x6e wa0x6e commented Mar 9, 2025

Toward https://github.com/snapshot-labs/workflow/issues/227
Toward https://github.com/snapshot-labs/workflow/issues/631

This PR adds validation to the space top level strategies.

Summary

It will ensure that:

  • top level strategies are existing strategies
  • top level strategies are not disabled

On both cases, it will return an error, preventing:

  • saving space settings with inexistent or disabled strategies
  • creating proposals when space settings contains inexistent or disabled strategies

The only disabled strategies right now is multichain

Changes

In order to accomplish the strategies validation, we needed to have a source of truth for valid and enabled strategies. This list will come from https://score.snapshot.org/api/strategies , and there is a new file which will poll and refresh a strategies object regularly, like on the hub.
The infinite loop is started on express.js app start, and will end gracefully on shutdown.

The validateSpaceSettings function, which was located in writer/settings.ts, has been extracted into its own file, in helper/validation.ts, since this function was also used by writer/proposal.ts
It makes more sense to have this common function inside a helper, which is also simplifying the test considerably, and avoiding duplicates tests.

Finally, there's been some refactoring in the tests, to:

  • extract validateSpaceSettings into its own file
  • remove duplicates tests in settings.tests.ts and proposals.test.ts

Test

  • Use SDK to send space setting with inexisting strategies
  • It should return error
  • Use SDK to send space setting with multichain strategy
  • It should return error

If not using an SDK, you can:

  • update your to use multichain strategies on production
  • connect your sx-ui to this sequencer branch
  • create a proposal on your space
  • sequencer should return an error
  • update your spaces settings (keep the multichain strategy)
  • sequencer should return an error

@wa0x6e wa0x6e requested a review from Copilot March 9, 2025 22:37
Copilot

This comment was marked as outdated.

@wa0x6e wa0x6e force-pushed the feat-validate-space-strategies branch from 0188a18 to 5d8276d Compare March 9, 2025 22:58
@wa0x6e wa0x6e force-pushed the feat-validate-space-strategies branch from 5d8276d to fac6311 Compare March 9, 2025 23:15
@wa0x6e wa0x6e marked this pull request as ready for review March 10, 2025 17:49
@wa0x6e wa0x6e requested a review from ChaituVR March 11, 2025 11:45
ChaituVR
ChaituVR previously approved these changes Mar 14, 2025
Copy link
Member

@ChaituVR ChaituVR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utAck, other than above this looks good

ChaituVR and others added 21 commits August 13, 2025 02:34
* fix: duplicate request preventor for EIP-1271 requests

* fix test
* chore: allows 50 follows for turbo users

* chore: allows 50 follows for turbo users
* fix: all copeland on production

* fix: remove unused variable
Updated mantle to mnt as it's new NetworkID for it.
…524)

Bumps [@snapshot-labs/snapshot.js](https://github.com/snapshot-labs/snapshot.js) from 0.12.54 to 0.12.55.
- [Release notes](https://github.com/snapshot-labs/snapshot.js/releases)
- [Commits](snapshot-labs/snapshot.js@v0.12.54...v0.12.55)

---
updated-dependencies:
- dependency-name: "@snapshot-labs/snapshot.js"
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…525)

Bumps [@snapshot-labs/snapshot.js](https://github.com/snapshot-labs/snapshot.js) from 0.12.55 to 0.12.56.
- [Release notes](https://github.com/snapshot-labs/snapshot.js/releases)
- [Commits](snapshot-labs/snapshot.js@v0.12.55...v0.12.56)

---
updated-dependencies:
- dependency-name: "@snapshot-labs/snapshot.js"
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…526)

Bumps [@snapshot-labs/snapshot.js](https://github.com/snapshot-labs/snapshot.js) from 0.12.56 to 0.12.57.
- [Release notes](https://github.com/snapshot-labs/snapshot.js/releases)
- [Commits](snapshot-labs/snapshot.js@v0.12.56...v0.12.57)

---
updated-dependencies:
- dependency-name: "@snapshot-labs/snapshot.js"
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
)

* feat: sync spaces custom domain with walletconnect allowed origins

* refactor: better variable name

* Update .env.example

Co-authored-by: Less <[email protected]>

* chore: only use domain without protocol prefix

---------

Co-authored-by: Chaitanya <[email protected]>
Co-authored-by: Less <[email protected]>
* feat: prevent non-premium networks

* add testcases

* Improve network filtering and fix test case
* feat: use correct network when fetching shib space controller

* chore: use network realm instead of chain id

* fix: allow network param override
…531)

Bumps [@snapshot-labs/snapshot.js](https://github.com/snapshot-labs/snapshot.js) from 0.12.57 to 0.12.58.
- [Release notes](https://github.com/snapshot-labs/snapshot.js/releases)
- [Commits](snapshot-labs/snapshot.js@v0.12.57...v0.12.58)

---
updated-dependencies:
- dependency-name: "@snapshot-labs/snapshot.js"
  dependency-version: 0.12.58
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* feat: add turbo tracking every 10s

* fix: correct syntax for GetSpaces query

* ci: deactivate schnaps for tests

* refactor: rename to turbo_expiration

* chore: remove quotes around url in .env.example

Co-authored-by: Chaitanya <[email protected]>

* refactor: only track if SCHNAPS_API_URL is set

* chore: remove log

* chore: remove dead env var

Co-authored-by: Wan <[email protected]>

* refactor: move while loop out of if condition

Co-authored-by: Wan <[email protected]>

* lint: fix

* chore: remove track turbo status

* ci: move loop in if condition

* refactor: move while loop out of if condition

* fix: process only relevant spaces for the instance network

* fix: use sentry to capture errors

* fix: use fech with keep alive

* fix: fix invalid comparison

* docs: add note about future feature

* fix: only update `turbo_expiration` field

* fix: improve sql query

---------

Co-authored-by: Chaitanya <[email protected]>
Co-authored-by: Wan <[email protected]>
…532)

Bumps [@snapshot-labs/snapshot.js](https://github.com/snapshot-labs/snapshot.js) from 0.12.58 to 0.12.59.
- [Release notes](https://github.com/snapshot-labs/snapshot.js/releases)
- [Commits](snapshot-labs/snapshot.js@v0.12.58...v0.12.59)

---
updated-dependencies:
- dependency-name: "@snapshot-labs/snapshot.js"
  dependency-version: 0.12.59
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…533)

Bumps [@snapshot-labs/snapshot.js](https://github.com/snapshot-labs/snapshot.js) from 0.12.59 to 0.12.60.
- [Release notes](https://github.com/snapshot-labs/snapshot.js/releases)
- [Commits](snapshot-labs/snapshot.js@v0.12.59...v0.12.60)

---
updated-dependencies:
- dependency-name: "@snapshot-labs/snapshot.js"
  dependency-version: 0.12.60
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* fix: infer turbo status from turbo expiration date

* fix: update schema.sql
dependabot bot and others added 16 commits August 13, 2025 02:37
)

Bumps [@snapshot-labs/snapshot.js](https://github.com/snapshot-labs/snapshot.js) from 0.14.1 to 0.14.2.
- [Release notes](https://github.com/snapshot-labs/snapshot.js/releases)
- [Commits](snapshot-labs/snapshot.js@v0.14.1...v0.14.2)

---
updated-dependencies:
- dependency-name: "@snapshot-labs/snapshot.js"
  dependency-version: 0.14.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
)

Bumps [@snapshot-labs/snapshot.js](https://github.com/snapshot-labs/snapshot.js) from 0.14.2 to 0.14.4.
- [Release notes](https://github.com/snapshot-labs/snapshot.js/releases)
- [Commits](snapshot-labs/snapshot.js@v0.14.2...v0.14.4)

---
updated-dependencies:
- dependency-name: "@snapshot-labs/snapshot.js"
  dependency-version: 0.14.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- Add validation to prevent circular references in space hierarchy
- Reject when space.parent equals the space ID
- Reject when space.children array contains the space ID
- Add comprehensive tests for parent/child validation
- Ensure proper error messages for validation failures
)

Bumps [@snapshot-labs/snapshot.js](https://github.com/snapshot-labs/snapshot.js) from 0.14.4 to 0.14.5.
- [Release notes](https://github.com/snapshot-labs/snapshot.js/releases)
- [Commits](snapshot-labs/snapshot.js@v0.14.4...v0.14.5)

---
updated-dependencies:
- dependency-name: "@snapshot-labs/snapshot.js"
  dependency-version: 0.14.5
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* feat: add support for sonic domain name

* fix: fix wrong sonic network
)

Bumps [@snapshot-labs/snapshot.js](https://github.com/snapshot-labs/snapshot.js) from 0.14.5 to 0.14.6.
- [Release notes](https://github.com/snapshot-labs/snapshot.js/releases)
- [Commits](snapshot-labs/snapshot.js@v0.14.5...v0.14.6)

---
updated-dependencies:
- dependency-name: "@snapshot-labs/snapshot.js"
  dependency-version: 0.14.6
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Wan <[email protected]>
Bumps @snapshot-labs/pineapple from 1.1.0 to 1.2.0.

---
updated-dependencies:
- dependency-name: "@snapshot-labs/pineapple"
  dependency-version: 1.2.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Wan <[email protected]>
)

Bumps [@snapshot-labs/snapshot.js](https://github.com/snapshot-labs/snapshot.js) from 0.14.6 to 0.14.7.
- [Release notes](https://github.com/snapshot-labs/snapshot.js/releases)
- [Commits](snapshot-labs/snapshot.js@v0.14.6...v0.14.7)

---
updated-dependencies:
- dependency-name: "@snapshot-labs/snapshot.js"
  dependency-version: 0.14.7
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@wa0x6e wa0x6e force-pushed the feat-validate-space-strategies branch from f688b47 to b0e08e3 Compare August 14, 2025 23:00
@wa0x6e wa0x6e force-pushed the feat-validate-space-strategies branch from b0e08e3 to 18c9919 Compare August 14, 2025 23:19
@wa0x6e wa0x6e requested a review from Copilot August 14, 2025 23:44
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds validation for space strategies to ensure they are existing and enabled strategies. The validation prevents saving space settings or creating proposals with invalid strategies, specifically targeting the disabled "multichain" strategy.

Key changes:

  • Extracts strategy validation logic into a new helper module with centralized validation
  • Implements strategy polling from the score API to maintain a source of truth for valid strategies
  • Refactors tests to use mocked validation functions and removes duplicate test code

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/helpers/validation.ts New validation helper containing extracted validateSpaceSettings function with strategy validation
src/helpers/strategies.ts New module that polls strategy data from score API and maintains strategies state
src/index.ts Adds strategy refresh initialization and graceful shutdown handling
src/writer/settings.ts Removes validation logic and imports from new validation helper
src/writer/proposal.ts Updates to use validation helper instead of local validation
test/unit/helpers/validation.test.ts New comprehensive test suite for validation logic
test/unit/writer/settings.test.ts Refactored to mock validation helper and remove duplicate tests
test/unit/writer/proposal.test.ts Simplified to use mocked validation helper
test/integration/ingestor.test.ts Adds strategy refresh initialization for integration tests

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@wa0x6e wa0x6e requested a review from ChaituVR August 15, 2025 00:27
@wa0x6e
Copy link
Contributor Author

wa0x6e commented Aug 15, 2025

The PR had been refactored since last review, to fetch and cache the strategies list, like on the hub, instead of getting it on each verify() call.

Also some refactoring to extract validateSpaceSetting into a helper, for easier testing.

See the updated PR description

@wa0x6e wa0x6e dismissed ChaituVR’s stale review August 15, 2025 00:29

PR updated substantially

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants